Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

643 AEL02 [Spec Added] #655

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open

643 AEL02 [Spec Added] #655

wants to merge 48 commits into from

Conversation

BFalquet
Copy link
Contributor

@BFalquet BFalquet commented Sep 11, 2023 •
edited
Loading

close #643
close #642
close #420
close #428
close #429
close #452

sibling PR in dunlin for improved whisker: insightsengineering/dunlin#126

thank you for the review

@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2023 •
edited
Loading

🧪 Test coverage: 97.15%

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  -------------------------------------------------------------------------------
R/ael01_nollt.R                 18       1  94.44%   62
R/ael02.R                       38       0  100.00%
R/ael03.R                       45       0  100.00%
R/ael04.R                       36       0  100.00%
R/aet01_aesi.R                 148       1  99.32%   211
R/aet01.R                       98       1  98.98%   168
R/aet02.R                       44       0  100.00%
R/aet03.R                       76       0  100.00%
R/aet04.R                       88       0  100.00%
R/aet05_all.R                   11       0  100.00%
R/aet05.R                       39       1  97.44%   106
R/aet10.R                       42       0  100.00%
R/assertions.R                  99       6  93.94%   88-93
R/cfbt01.R                     107       0  100.00%
R/checks.R                      14       0  100.00%
R/chevron_tlg-S4class.R         18       0  100.00%
R/chevron_tlg-S4methods.R      136       4  97.06%   67, 199-207, 352
R/cml02a_gl.R                   19       0  100.00%
R/cmt01a.R                      52       0  100.00%
R/coxt01.R                      49       1  97.96%   129
R/dml01.R                       16       0  100.00%
R/dml02.R                       25       0  100.00%
R/dmt01.R                       36       0  100.00%
R/dsl01.R                       28       0  100.00%
R/dsl02.R                       36       0  100.00%
R/dst01.R                       94       0  100.00%
R/dtht01.R                     102       6  94.12%   50, 54-58
R/egt02.R                       35       0  100.00%
R/egt03.R                       80       1  98.75%   153
R/egt05_qtcat.R                 78       0  100.00%
R/ext01.R                       59       1  98.31%   50
R/fstg01.R                      44       0  100.00%
R/fstg02.R                      49       0  100.00%
R/kmg01.R                       29       0  100.00%
R/lbt04.R                       90       0  100.00%
R/lbt05.R                       66       5  92.42%   123-128
R/lbt06.R                       70       3  95.71%   141-144
R/lbt07.R                       93       0  100.00%
R/lbt14.R                       64       0  100.00%
R/lbt15.R                       20       0  100.00%
R/mht01.R                       54       0  100.00%
R/mng01.R                       75       2  97.33%   104-105
R/pdl01.R                       22       0  100.00%
R/pdt01.R                       60       0  100.00%
R/pdt02.R                       68       0  100.00%
R/rmpt01.R                      59      11  81.36%   87-96, 131
R/rmpt06.R                     134       3  97.76%   209-212
R/rspt01.R                      73       3  95.89%   155-158
R/rtables_utils.R              319      23  92.79%   51, 68, 119, 243, 264, 387-394, 461, 476-478, 480, 549-555, 565, 586
R/standard_rules.R              13       0  100.00%
R/ttet01.R                     117       3  97.44%   198-201
R/utils.R                      180      23  87.22%   141, 156, 175-177, 296-299, 303-306, 333, 337, 349, 351, 363, 380-382, 446, 467
R/vst02.R                       45       1  97.78%   108
TOTAL                         3510     100  97.15%

Diff against main

Filename              Stmts    Miss  Cover
------------------  -------  ------  --------
R/ael01_nollt.R          -3       0  -0.79%
R/ael02.R               +38       0  +100.00%
R/ael03.R               +45       0  +100.00%
R/ael04.R               +36       0  +100.00%
R/aet01_aesi.R           -2       0  -0.01%
R/aet01.R                +4       0  +0.04%
R/aet02.R                -6       0  +100.00%
R/aet03.R                -1       0  +100.00%
R/aet04.R                -1       0  +100.00%
R/aet05.R                -1       0  -0.06%
R/aet10.R                -1       0  +100.00%
R/cml02a_gl.R           +19       0  +100.00%
R/cmt01a.R               -6      -6  +10.34%
R/coxt01.R               +1       0  +0.04%
R/dml01.R               +16       0  +100.00%
R/dml02.R               +25       0  +100.00%
R/dmt01.R                +7       0  +100.00%
R/dsl01.R               +28       0  +100.00%
R/dsl02.R               +36       0  +100.00%
R/dst01.R                -1       0  +100.00%
R/dtht01.R               -1       0  -0.06%
R/egt02.R                -2       0  +100.00%
R/egt03.R                +3       0  +0.05%
R/egt05_qtcat.R          -1       0  +100.00%
R/ext01.R                -1       0  -0.03%
R/fstg01.R               -6       0  +100.00%
R/fstg02.R               -6       0  +100.00%
R/lbt04.R                -1       0  +100.00%
R/lbt05.R                -1       0  -0.11%
R/lbt06.R                -2       0  -0.12%
R/lbt07.R                -1       0  +100.00%
R/lbt14.R                -2       0  +100.00%
R/mht01.R               -17       0  +100.00%
R/mng01.R               -21      +1  -1.62%
R/pdl01.R               +22       0  +100.00%
R/pdt02.R                -1       0  +100.00%
R/rmpt01.R               -6       0  -1.72%
R/rmpt06.R             +134      +3  +97.76%
R/rtables_utils.R       +19      +4  -0.88%
R/standard_rules.R       +2       0  +100.00%
R/utils.R               +96     +23  -12.78%
R/vst02.R                -1       0  -0.05%
TOTAL                  +439     +25  -0.41%

Results for commit: 658dd60

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2023 •
edited
Loading

Unit Tests Summary

    1 files    62 suites   4m 11s ⏱️
257 tests 189 ✔️   68 💤 0 ❌
505 runs  320 ✔️ 185 💤 0 ❌

Results for commit 658dd60.

♻️ This comment has been updated with latest results.

@BFalquet BFalquet requested a review from barnett11 September 15, 2023 09:46
clarkliming
clarkliming reviewed Sep 18, 2023
View reviewed changes
R/ael02.R Outdated Show resolved Hide resolved
clarkliming
clarkliming reviewed Sep 18, 2023
View reviewed changes
R/ael02.R Outdated Show resolved Hide resolved
clarkliming
clarkliming reviewed Sep 18, 2023
View reviewed changes
R/ael02.R Outdated Show resolved Hide resolved
clarkliming
clarkliming reviewed Sep 21, 2023
View reviewed changes
R/utils.R Outdated Show resolved Hide resolved
clarkliming
clarkliming reviewed Sep 21, 2023
View reviewed changes
R/utils.R Outdated Show resolved Hide resolved
clarkliming
clarkliming reviewed Sep 21, 2023
View reviewed changes
R/dsl02.R Outdated Show resolved Hide resolved
barnett11
barnett11 reviewed Sep 21, 2023
View reviewed changes
R/ael02.R Outdated Show resolved Hide resolved
barnett11
barnett11 reviewed Sep 21, 2023
View reviewed changes
R/ael02.R Outdated Show resolved Hide resolved
@BFalquet BFalquet requested a review from barnett11 September 27, 2023 08:16
barnett11
barnett11 requested changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@barnett11 barnett11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DML02:

  • Center/Patient should be first column
  • Ordering to be by date of randomization
  • Label of Center/Patient ID mislabelled as Patients

@barnett11
Copy link
Contributor

DML01:

  • Why is Study ID shown?
  • Treatment Group should be primary sorting column (ideally at top of listing as template)
  • Exclude Age group as default
  • Patient ID and not Patients ID

@barnett11
Copy link
Contributor

CML02A_GL:
Erroring with below,

Error in h(simpleError(msg, call)) : 
  error in evaluating the argument 'x' in selecting a method for function 'toString': values must be length 1,
 but FUN(X[[1]]) result is length 0```

@barnett11
Copy link
Contributor

AEL02:

  • Patients ID label
  • Treatment group should be primary sort order, ideally at top of listing
  • Action Taken - I'm getting an 8 for terms not identifiable - but 8 is not something we recognise. If text is not recognised in format can we just print the text?

@BFalquet BFalquet mentioned this pull request Oct 6, 2023
Open
@BFalquet
Copy link
Contributor Author

BFalquet commented Oct 6, 2023

DML01:

  • Why is Study ID shown?
  • Treatment Group should be primary sorting column (ideally at top of listing as template)
  • Exclude Age group as default
  • Patient ID and not Patients ID
  • Study ID has been removed
  • Treatment group now sort the listing (not possible to put it on to left yet, so it is in left column for now)
  • Age group has been excluded
  • patient label has been corrected.

@BFalquet
Copy link
Contributor Author

BFalquet commented Oct 6, 2023

DML02:

  • Center/Patient should be first column
  • Ordering to be by date of randomization
  • Label of Center/Patient ID mislabelled as Patients
  • Label corrected
  • Center/Patient is now the first column
  • Add a little mechanism to sort by randomization date.

@BFalquet BFalquet added on hold and removed onhold labels Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clarkliming clarkliming clarkliming left review comments

@barnett11 barnett11 barnett11 requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
chevron on hold
Projects
None yet
Milestone
No milestone
3 participants
@BFalquet @barnett11 @clarkliming